Skip to content

Conversation

@damrobi
Copy link
Collaborator

@damrobi damrobi commented Feb 10, 2026

Content

This PR includes a fix to the implementation of the unique schnorr signature that differs between cpu and circuit. This fix mainly consists in modifying the input to the sign and verify functions of the unique_schnorr_signature module to take BaseFieldElements instead of arbitrary bytes. The bytes that were previously handled within the sign function will be treated outside of the signature in a later PR.
Warning: This PR does not include the proper handling of the message bytes and merkle root before giving them to the signature function!

Content of the PR:

  • Changed hash_to_projective_point to have BaseFieldElement as inputs
  • Modified inputs of sign and verify functions to take &[BaseFieldElement] instead of &[u8]
  • Exposed BaseFieldElement to the benches
  • Added From<&[u8;32]> implementation to BaseFieldElement for testing only

Pre-submit checklist

  • Branch
    • Tests are provided (if possible)
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • All check jobs of the CI have succeeded
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • No new TODOs introduced

Comments

Issue(s)

Relates to #2993

@github-actions
Copy link

github-actions bot commented Feb 10, 2026

Test Results

    5 files  ±0    172 suites  ±0   39m 52s ⏱️ +10s
2 513 tests ±0  2 513 ✅ +7  0 💤 ±0  0 ❌  - 7 
7 783 runs  ±0  7 783 ✅ +7  0 💤 ±0  0 ❌  - 7 

Results for commit 5d48fba. ± Comparison against base commit 78d4c0d.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hjeljeli32 hjeljeli32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damrobi
Thanks a lot for the quick turnaround on this PR 🙏
This looks really good to me and is fully aligned with the decisions we discussed yesterday.
I left a couple of small comments regarding doc comments that are now slightly outdated, but nothing blocking from my side.
Great work 👍

@@ -63,18 +62,10 @@ impl ProjectivePoint {
/// Hashes input bytes to a projective point on the Jubjub curve
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damrobi
The doc comment on hash_to_projective_point is now outdated.

/// and works with the Jubjub elliptic curve and the Poseidon hash function.
///
/// Input:
/// - a message: some bytes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damrobi The doc comment needs to be updated.

///
/// Input:
/// - a Unique Schnorr signature
/// - a message: some bytes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@damrobi doc comment needs to be updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants